Fix multiple partition SQL syntax for MySQL by dereuromark · Pull Request #988 · cakephp/migrations

dereuromark · 2026-01-06T01:51:22Z

Summary

Fixes invalid MySQL syntax when adding multiple partitions to an existing table
MySQL requires ADD PARTITION (PARTITION p1 ..., PARTITION p2 ...) not ADD PARTITION (...), ADD PARTITION (...)
Similarly batches DROP PARTITION statements for efficiency

Changes Made

Modified executeActions() in AbstractAdapter.php to batch partition actions together
Added getAddPartitionsInstructions() and getDropPartitionsInstructions() methods to AbstractAdapter
Overrode these methods in MysqlAdapter to generate correct batched SQL syntax
Added gatherPartitions() to Plan.php to properly gather partition actions
Added $partitions property and updated updatesSequence()/inverseUpdatesSequence() in Plan.php

Why PR #987 Tests Didn't Catch This

The tests in PR #987 only add/drop a single partition at a time. With a single partition, the generated SQL is valid:

ALTER TABLE `foo` ADD PARTITION (PARTITION `p2024` VALUES LESS THAN ('2025-01-01'));

The bug only manifests when adding multiple partitions:

-- Invalid (before fix):
ALTER TABLE `foo` ADD PARTITION (...), ADD PARTITION (...);

-- Valid (after fix):
ALTER TABLE `foo` ADD PARTITION (PARTITION p1 ..., PARTITION p2 ...);

Extensive Tests Added

testAddSinglePartitionToExistingTable - verifies single partition add works
testAddMultiplePartitionsToExistingTable - main test for the fix
testDropSinglePartitionFromExistingTable - verifies single partition drop works
testDropMultiplePartitionsFromExistingTable - verifies batched DROP PARTITION
testAddMultipleListPartitionsToExistingTable - tests LIST partitioning
testAddPartitionsWithMaxvalue - tests MAXVALUE handling in batched adds

Refs #986

When adding multiple partitions to an existing table, MySQL requires: ALTER TABLE foo ADD PARTITION (PARTITION p1 ..., PARTITION p2 ...) Previously, each AddPartition action generated its own ADD PARTITION clause, which when joined with commas resulted in invalid SQL: ALTER TABLE foo ADD PARTITION (...), ADD PARTITION (...) This fix: - Batches AddPartition actions together in executeActions() - Adds new getAddPartitionsInstructions() method to AbstractAdapter with a default implementation that calls the single partition method - Overrides getAddPartitionsInstructions() in MysqlAdapter to generate correct batched SQL: ADD PARTITION (PARTITION p1, PARTITION p2) - Similarly batches DropPartition actions for efficiency - Adds gatherPartitions() to Plan.php to properly gather partition actions - Includes extensive tests for single/multiple partition add/drop scenarios Refs #986

dereuromark · 2026-01-06T02:30:34Z

Edge cases analyzed:

Edge Case	Status	Notes
Single partition add	✅	testAddSinglePartitionToExistingTable
Multiple partitions add	✅	testAddMultiplePartitionsToExistingTable - main fix
Single partition drop	✅	testDropSinglePartitionFromExistingTable
Multiple partitions drop	✅	testDropMultiplePartitionsFromExistingTable
LIST partitions	✅	testAddMultipleListPartitionsToExistingTable
MAXVALUE handling	✅	testAddPartitionsWithMaxvalue
Combined partition+column	✅	testCombinedPartitionAndColumnOperations - added
PostgreSQL	✅	Uses separate CREATE TABLE statements, batching not needed
Empty partition list	✅	Handled with if (empty($partitions)) checks

Key observations:

The fix correctly batches partition operations in MySQL
PostgreSQL doesn't need batching (uses CREATE TABLE ... PARTITION OF)
Default AbstractAdapter implementation falls back to single-partition calls for other adapters
The combined test verifies partition+column operations work together

jamisonbryant

This works now for me. Generated query:

ALTER TABLE `foo_partitioned` ADD PARTITION (PARTITION `p2023` VALUES LESS THAN ('2024-01-01'), PARTITION `p2024` VALUES LESS THAN ('2025-01-01'), PARTITION `p2025_01` VALUES LESS THAN ('2025-02-01'), PARTITION `p2025_02` VALUES LESS THAN ('2025-03-01'), PARTITION `p2025_03` VALUES LESS THAN ('2025-04-01'), PARTITION `p2025_04` VALUES LESS THAN ('2025-05-01'), PARTITION `p2025_05` VALUES LESS THAN ('2025-06-01'), PARTITION `p2025_06` VALUES LESS THAN ('2025-07-01'), PARTITION `p2025_07` VALUES LESS THAN ('2025-08-01'), PARTITION `p2025_08` VALUES LESS THAN ('2025-09-01'), PARTITION `p2025_09` VALUES LESS THAN ('2025-10-01'), PARTITION `p2025_10` VALUES LESS THAN ('2025-11-01'), PARTITION `p2025_11` VALUES LESS THAN ('2025-12-01'), PARTITION `p2025_12` VALUES LESS THAN ('2026-01-01'), PARTITION `pmax` VALUES LESS THAN MAXVALUE);

Code changes look good as well. Shall I close my PR?

markstory · 2026-01-06T04:51:40Z

tests/TestCase/Db/Adapter/MysqlAdapterTest.php

+        $table->addPartitionToExisting('p2023', '2024-01-01')
+            ->addPartitionToExisting('p2024', '2025-01-01')
+            ->addPartitionToExisting('p2025', '2026-01-01')


What is the error when one uses addPartitionToExisting() on a new table, and addPartition() to an existing table?

Errors weren't being thrown by the plugin - it's that no SQL (or wrong SQL) is generated for these edge cases.

addPartitionToExisting() on a new table:
When you call create(), the AddPartition action gets filtered out because the table is in tableCreates. The table is created without any partitioning - no SQL generated for the partition.

addPartition() on an existing table:
Two sub-cases:

Without calling partitionBy() first: RuntimeException: 'Must call partitionBy() before addPartition()'

With partitionBy() called first: This generates a SetPartitioning action which produces:
ALTER TABLE existing_table PARTITION BY RANGE (column) (PARTITION p1 VALUES LESS THAN (...))

This sets/replaces the entire partition scheme rather than adding a single partition to an existing scheme - wrong SQL for the intent.

markstory · 2026-01-06T04:55:36Z

src/Db/Adapter/AbstractAdapter.php

+        foreach ($partitions as $partition) {
+            $instructions->merge($this->getAddPartitionInstructions($table, $partition));
+        }


Do we need an abstraction for both the collection and the single partition expression? It seems like the collection + single methods are always implemented together and are only used in tandem. Could we have only a single abstract method then?

You're right. Looking at it now:

The collection method is the only entry point (called from executeActions)

MySQL's collection method doesn't even use the single method - it uses a private getAddPartitionSql() helper

The single method only exists for AbstractAdapter's default loop implementation

I can simplify to just the collection methods (getAddPartitionsInstructions / getDropPartitionsInstructions). Each adapter can implement the batching however it needs internally - MySQL with its combined syntax, PostgreSQL with a simple loop. The single-partition logic becomes a private implementation detail rather than part of the abstract contract.

Changes made in #989

dereuromark · 2026-01-06T12:04:21Z

We can close in favor of #989 then

dereuromark marked this pull request as draft January 6, 2026 01:53

dereuromark added 2 commits January 6, 2026 02:54

Fix coding standard - use single quotes

31aea48

Trigger CI re-run

fd05bf2

dereuromark mentioned this pull request Jan 6, 2026

Add support for Partitions to plans #987

Closed

Add test for combined partition and column operations

e978c23

dereuromark marked this pull request as ready for review January 6, 2026 02:30

jamisonbryant self-requested a review January 6, 2026 02:39

jamisonbryant approved these changes Jan 6, 2026

View reviewed changes

jamisonbryant mentioned this pull request Jan 6, 2026

Add support for partitionBy() + update() on existing tables #989

Merged

markstory reviewed Jan 6, 2026

View reviewed changes

jamisonbryant closed this Jan 6, 2026

dereuromark deleted the fix-multiple-partition-sql-syntax branch January 27, 2026 16:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix multiple partition SQL syntax for MySQL#988

Fix multiple partition SQL syntax for MySQL#988
dereuromark wants to merge 4 commits into5.xfrom
fix-multiple-partition-sql-syntax

dereuromark commented Jan 6, 2026

Uh oh!

dereuromark commented Jan 6, 2026

Uh oh!

jamisonbryant left a comment

Uh oh!

markstory Jan 6, 2026

Uh oh!

jamisonbryant Jan 6, 2026

Uh oh!

markstory Jan 6, 2026

Uh oh!

jamisonbryant Jan 6, 2026

Uh oh!

jamisonbryant Jan 6, 2026

Uh oh!

dereuromark commented Jan 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

dereuromark commented Jan 6, 2026

Summary

Changes Made

Why PR #987 Tests Didn't Catch This

Extensive Tests Added

Uh oh!

dereuromark commented Jan 6, 2026

Uh oh!

jamisonbryant left a comment

Choose a reason for hiding this comment

Uh oh!

markstory Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

jamisonbryant Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

markstory Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

jamisonbryant Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

jamisonbryant Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

dereuromark commented Jan 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants